Skip to content

ResNet Tests corrected for netfx - #2411

Merged
Anipik merged 5 commits into
dotnet:masterfrom
Anipik:fixResnetTest
Feb 7, 2019
Merged

ResNet Tests corrected for netfx#2411
Anipik merged 5 commits into
dotnet:masterfrom
Anipik:fixResnetTest

Conversation

@Anipik

@Anipik Anipik commented Feb 5, 2019

Copy link
Copy Markdown
Contributor

Fixes #2373

The behavior of GetExecutingAssembly is different on netFramework. It return a temp path on .netFramework

This problem also occurred while enabling tests for netfx.

@codecov

codecov Bot commented Feb 5, 2019

Copy link
Copy Markdown

Codecov Report

Merging #2411 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2411      +/-   ##
==========================================
- Coverage   71.26%   71.22%   -0.04%     
==========================================
  Files         785      786       +1     
  Lines      140869   140982     +113     
  Branches    16102    16116      +14     
==========================================
+ Hits       100384   100413      +29     
- Misses      36021    36098      +77     
- Partials     4464     4471       +7
Flag Coverage Δ
#Debug 71.22% <100%> (-0.04%) ⬇️
#production 67.58% <100%> (-0.04%) ⬇️
#test 85.28% <ø> (-0.02%) ⬇️

Comment thread src/Microsoft.ML.DnnImageFeaturizer.ResNet18/ResNet18Extension.cs Outdated
Comment thread src/Microsoft.ML.DnnImageFeaturizer.ResNet18/ResNet18Extension.cs Outdated
@eerhardt eerhardt assigned vaeksare and unassigned vaeksare Feb 5, 2019
@eerhardt
eerhardt requested a review from vaeksare February 5, 2019 18:21
@Anipik

Anipik commented Feb 5, 2019

Copy link
Copy Markdown
Contributor Author

@Ivanidzo4ka @sfilipi @justinormont can you take a look at this PR ?

Comment thread src/Microsoft.ML.DnnImageFeaturizer.AlexNet/AlexNetExtension.cs Outdated
@jignparm

jignparm commented Feb 6, 2019

Copy link
Copy Markdown
Contributor

Can we enable the DNNImage tests in this PR to verify if the fix works in the the netfx leg? It doesn't look like the DNNImage tests are part of the netfx test run currently.

@Anipik

Anipik commented Feb 6, 2019

Copy link
Copy Markdown
Contributor Author

Can we enable the DNNImage tests in this PR to verify if the fix works in the the netfx leg? It doesn't look like the DNNImage tests are part of the netfx test run currently.

There is an already open PR by a contributor to remove all conditional attributes #2402

I have tested these tests locally by just removing the conditional fact attribute, if there is any fall out, i will add the commit in #2402

…oving the common code to a new helper funciton
@Anipik

Anipik commented Feb 7, 2019

Copy link
Copy Markdown
Contributor Author

@eerhardt @jignparm @justinormont I addressed the required feedback

Comment thread src/Microsoft.ML.DnnImageFeaturizer.AlexNet/AssemblyPathHelpers.cs
Comment thread src/Microsoft.ML.DnnImageFeaturizer.AlexNet/AssemblyPathHelpers.cs Outdated
@Anipik
Anipik merged commit 3b1358e into dotnet:master Feb 7, 2019
@Anipik
Anipik deleted the fixResnetTest branch February 7, 2019 21:47
@ghost ghost locked as resolved and limited conversation to collaborators Mar 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants